Fix build break because of missing metdata #8364
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In the previous change I didn't notice that the binplacing logic depends on metadata passing in being preserved in the items being returned. Because of that, I revert the collection change from List to HashSet and instead manually iterate through the list. The binplacing logic depends on duplicates being allowed versus the inner build project logic explicitly doesn't want duplicates to avoid unnecessary inner builds, hence adding a switch for it.
safern
reviewed
Jan 20, 2022
|
|
||
| [Required] | ||
| public string RuntimeGraph { get; set; } | ||
|
|
||
| [Required] | ||
| public string[] SupportedTargetFrameworks { get; set; } | ||
|
|
||
| // Returns distinct items only. Compares the include values. Metadata is ignored. | ||
| public bool Distinct { get; set; } |
There was a problem hiding this comment.
Why do we need to turn this off or on and why can't the behavior just be that we always return a Distinct value?
There was a problem hiding this comment.
Details are in the top comment. The binplacing logic which also uses this task requires non distinct values where-as the DispatchToInnerBuild logic which is responsible for filtering out non applicable inner builds requires distinct values.
safern
approved these changes
Jan 20, 2022
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the previous change I didn't notice that the binplacing logic depends on metadata passing in being preserved in the items being returned. Because of that, I revert the collection change from List to HashSet and instead manually iterate through the list. The binplacing logic depends on duplicates being allowed versus the inner build project logic explicitly doesn't want duplicates to avoid unnecessary inner builds, hence adding a switch for it.
Tested this in dotnet/runtime for real this time. I didn't notice the break before as I tested a part of the libs subset in dotnet/runtime which wasn't affected.